This one is tricky since it needs to directly reference CurrencyId::Tranche, therefore breaking the genericness of the CurrencyId type. So either we make the type param be very strict on the cfg_types::CurrencyId enum type or we just reference it directly.

I am working on the currencyId enum anyways this week. We could make the TrancheCurrency a struct which connectors directly depends on and it will implement Into<CurrencyId>.

Other solution, we extend the PoolInspect trait and let it return TrancheCurrency and PoolCurrency for a pool, if it exists. Then you could simply use the returned Currency.

Ah yes that would be nice @mustermeiszer 😃

Which one do you prefer? I am still in the process of making up my mind around the changes to CurrencyId.

I have two options in my head:

First - structured currency types that are wrapped in enums

// Also implements `Into<CurrencyId`
struct TrancheCurrency<PoolId, TrancheId> {
   pool_id: PoolId, 
   tranche_id: TrancheId
}

impl<PoolId, TrancheId> TrancheCurrency<PoolId, TrancheId> {
   const DECIMALS: u128 = ???; // Need to get this from the registry
   const SYMBOL: &str =  ???; //Need to get this from the registry
}

// Also implements `Into<CurrencyId`
struct KSM;

impl KSM {
   const DECIMALS: u128 = primitives::currency_decimals::KSM;
   const SYMBOL: &str =  &"KSM";
}

// Also implements `Into<CurrencyId`
struct AUSD;

impl AUSD {
   const DECIMALS: u128 = primitives::currency_decimals::AUSD;
   const SYMBOL: &str =  &"AUSD";
}

// Currencies that can. beused in a pool as a base currency
pub enum PoolCurrency {
   KSM(KSM),
   AUSD(AUSD)
}

// The currencies one can invest into 
pub enum InvestmentId {
   Tranche(TrancheCurrency<PoolId, TrancheId>)
}

// The overarching CurrencyId enum that is only used by pallets
// that need knowledge of all currencies, e.g. orml_tokens
//
// MUST NOT be differentiate between runtimes, but could be
mod altair {
   enum CurrencyId {
     KSM(KSM),
     AUSD(AUSD),
     TrancheCurrency(TrancheCurrency)
   }
}

Second - Do not use structured currencies but rather have the enums convert it

// Currencies that can. beused in a pool as a base currency
pub enum PoolCurrency {
   KSM,
   AUSD
}

impl Into<CurrencyId> for PoolCurrency {
...
}

// The currencies one can invest into 
pub enum InvestmentId {
   Tranche(TrancheCurrency<PoolId, TrancheId>)
}

impl Into<CurrencyId> for InvestmentId {
...
}

@mustermeiszer do you mind creating an issue to discuss this? 🙏

FYI the first option was implemented in #995

Cool, thanks! @mustermeiszer I might ping you on this one :)

@NunoAlexandre do we still want to address this TODO here? I think it should be possible now that #995 was merged

Correct me if I'm wrong, but I think the get_tranche_token_price method would fail on line https://github.com/centrifuge/centrifuge-chain/blob/main/pallets/pools/src/impls.rs#L48 as well if the pool_id or tranche_id was invalid. So it might be more efficient to remove T::PoolInspect::tranche_exists, wdyt?

Hm yeah, if we do that we should change the error MissingPrice to something more general, otherwise it will sound like only the price of an non-existing tranche is missing. wdyt?

Yeah that makes sense, not entirely sure what the ideal naming is though. Any suggestions?

So, we need to check whether the tranche token exists in add_tranche whilst not needing to fetch the tranche price. This means that we need to have an error variant for TrancheNotFound for this case.

So I just renamed MissingPrice to MissingTranchePrice to be more specific it's Tranche-related and left a richer doc comment on what this error can mean.

FYI the first option was implemented in #995

@NunoAlexandre do we still want to address this TODO here? I think it should be possible now that #995 was merged